-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Inference Connector] Changed UI/UX due to the new RFC for the _inference/_service #203363
[Inference Connector] Changed UI/UX due to the new RFC for the _inference/_service #203363
Conversation
@@ -183,9 +710,6 @@ const openAiConnector = { | |||
model_id: 'gpt-4o', | |||
organization_id: 'test-org', | |||
}, | |||
taskTypeConfig: { | |||
user: 'elastic', | |||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@YulNaumenko To get this jest test passing, I had to remove this. Looks like user
used to be a task setting for this config but we're not setting it anymore. Is that ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's correct. Thank you for cleaning up the code.
label: 'API Key', | ||
required: true, | ||
sensitive: true, | ||
updatable: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updatable
is in the RFC so I added it to the hard-coded response schema but just set it to true
for now since it's not used anywhere in the UI. When I update the API on the Elasticsearch side, I will verify the updatability of each field and set appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
DROPDOWN = 'dropdown', | ||
CHECKABLE = 'checkable', | ||
} | ||
|
||
export interface SelectOption { | ||
label: string; | ||
value: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is icon
needed? I am importing SelectOption
from search-connectors
and icon
is not defined there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, icon is needed to display it in the select if it present
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
cc @YulNaumenko |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/12318938202 |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
Friendly reminder: Looks like this PR hasn’t been backported yet. |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ence/_service (elastic#203363) Related RFC https://docs.google.com/document/d/1DbWpqEKM-MJR2cSJNKLC7RcCNXXQ-_iUShsXKRFKXfk/edit?tab=t.0 ## Summary - removed Task Settings from the UI and schema definition, due to the discussion on Inference sync Dec 5th. - renamed provider to service - added name and description, use name for the service selector user friendly way - dropped options and display type dropdown select, use freeform text input instead - dropped `display` field type, renamed `tooltip` to the `description`. Properly updated `ConnectorConfigurationFormItems` in `x-pack/plugins/stack_connectors/public/connector_types/lib/dynamic_config/connector_configuration_form_items.tsx` UI with the updates: <img width="1281" alt="Screenshot 2024-12-08 at 10 09 52 PM" src="https://github.com/user-attachments/assets/fdb17dd4-c8e4-496b-85e7-03c363546b8e"> --------- Co-authored-by: Ying <[email protected]> (cherry picked from commit 5b10845)
…_inference/_service (#203363) (#204690) # Backport This will backport the following commits from `main` to `8.x`: - [[Inference Connector] Changed UI/UX due to the new RFC for the _inference/_service (#203363)](#203363) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Yuliia Naumenko","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-13T16:00:59Z","message":"[Inference Connector] Changed UI/UX due to the new RFC for the _inference/_service (#203363)\n\nRelated RFC\r\nhttps://docs.google.com/document/d/1DbWpqEKM-MJR2cSJNKLC7RcCNXXQ-_iUShsXKRFKXfk/edit?tab=t.0\r\n\r\n## Summary\r\n\r\n- removed Task Settings from the UI and schema definition, due to the\r\ndiscussion on Inference sync Dec 5th.\r\n- renamed provider to service\r\n- added name and description, use name for the service selector user\r\nfriendly way\r\n- dropped options and display type dropdown select, use freeform text\r\ninput instead\r\n- dropped `display` field type, renamed `tooltip` to the `description`.\r\nProperly updated `ConnectorConfigurationFormItems` in\r\n`x-pack/plugins/stack_connectors/public/connector_types/lib/dynamic_config/connector_configuration_form_items.tsx`\r\n\r\nUI with the updates:\r\n<img width=\"1281\" alt=\"Screenshot 2024-12-08 at 10 09 52 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/fdb17dd4-c8e4-496b-85e7-03c363546b8e\">\r\n\r\n---------\r\n\r\nCo-authored-by: Ying <[email protected]>","sha":"5b108453ffe823b5d559952f37da1030a79d3352","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport missing","v9.0.0","backport:version","v8.18.0"],"number":203363,"url":"https://github.com/elastic/kibana/pull/203363","mergeCommit":{"message":"[Inference Connector] Changed UI/UX due to the new RFC for the _inference/_service (#203363)\n\nRelated RFC\r\nhttps://docs.google.com/document/d/1DbWpqEKM-MJR2cSJNKLC7RcCNXXQ-_iUShsXKRFKXfk/edit?tab=t.0\r\n\r\n## Summary\r\n\r\n- removed Task Settings from the UI and schema definition, due to the\r\ndiscussion on Inference sync Dec 5th.\r\n- renamed provider to service\r\n- added name and description, use name for the service selector user\r\nfriendly way\r\n- dropped options and display type dropdown select, use freeform text\r\ninput instead\r\n- dropped `display` field type, renamed `tooltip` to the `description`.\r\nProperly updated `ConnectorConfigurationFormItems` in\r\n`x-pack/plugins/stack_connectors/public/connector_types/lib/dynamic_config/connector_configuration_form_items.tsx`\r\n\r\nUI with the updates:\r\n<img width=\"1281\" alt=\"Screenshot 2024-12-08 at 10 09 52 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/fdb17dd4-c8e4-496b-85e7-03c363546b8e\">\r\n\r\n---------\r\n\r\nCo-authored-by: Ying <[email protected]>","sha":"5b108453ffe823b5d559952f37da1030a79d3352"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203363","number":203363,"mergeCommit":{"message":"[Inference Connector] Changed UI/UX due to the new RFC for the _inference/_service (#203363)\n\nRelated RFC\r\nhttps://docs.google.com/document/d/1DbWpqEKM-MJR2cSJNKLC7RcCNXXQ-_iUShsXKRFKXfk/edit?tab=t.0\r\n\r\n## Summary\r\n\r\n- removed Task Settings from the UI and schema definition, due to the\r\ndiscussion on Inference sync Dec 5th.\r\n- renamed provider to service\r\n- added name and description, use name for the service selector user\r\nfriendly way\r\n- dropped options and display type dropdown select, use freeform text\r\ninput instead\r\n- dropped `display` field type, renamed `tooltip` to the `description`.\r\nProperly updated `ConnectorConfigurationFormItems` in\r\n`x-pack/plugins/stack_connectors/public/connector_types/lib/dynamic_config/connector_configuration_form_items.tsx`\r\n\r\nUI with the updates:\r\n<img width=\"1281\" alt=\"Screenshot 2024-12-08 at 10 09 52 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/fdb17dd4-c8e4-496b-85e7-03c363546b8e\">\r\n\r\n---------\r\n\r\nCo-authored-by: Ying <[email protected]>","sha":"5b108453ffe823b5d559952f37da1030a79d3352"}},{"branch":"8.x","label":"v8.18.0","labelRegex":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Yuliia Naumenko <[email protected]>
Related RFC https://docs.google.com/document/d/1DbWpqEKM-MJR2cSJNKLC7RcCNXXQ-_iUShsXKRFKXfk/edit?tab=t.0
Summary
display
field type, renamedtooltip
to thedescription
. Properly updatedConnectorConfigurationFormItems
inx-pack/plugins/stack_connectors/public/connector_types/lib/dynamic_config/connector_configuration_form_items.tsx
UI with the updates: